Skip to content

Conversation

weaverryan
Copy link
Member

Q A
Bug fix? no
New feature? yes
Tickets Fixes #86
License MIT

This takes the work from #126 and rebases so we can finish on the 2.x branch.

@tgalopin
Copy link
Contributor

tgalopin commented Dec 2, 2021

There seems to be a missing browser feature required by Chartjs 3 (ResizeObserver), perhaps would it be necessary to add a new polyfill.

@weaverryan
Copy link
Member Author

There seems to be a missing browser feature required by Chartjs 3 (ResizeObserver), perhaps would it be necessary to add a new polyfill.

You're right - it WAS missing in the test environment. I polyfilled it there. But I don't think we need to polyfill it for the users, if that's what you meant (let me know if that IS what you were thinking and let's chat).

I have just one more tweak on this one - it's in the "externals" of the built file (if you build the file right now, the dist file will "contain" chartjs instead of just having the import line). I know the fix - I just need to make a nice addition to the build system for it.

@weaverryan
Copy link
Member Author

This is now ready!

@tgalopin
Copy link
Contributor

tgalopin commented Dec 4, 2021

Could you squash your commits? I can't do it automatically due to two authors :)

@Huluti
Copy link
Contributor

Huluti commented Dec 5, 2021

ChartJS is now at v3.6.2. Is it something to change in this PR since it's based on v3.4.1?

@weaverryan
Copy link
Member Author

The 3.4.1 only means that it's the minimum version. In reality, if you installed this, you would have ^3.4.1 in your package.json, but yarn/npm would install the latest version 3 (so 3.6.2). It's really not a big deal one way or another, but I'd say just keep it.

I've got it rebased to only 2 commits - that's how (I think) we should keep it.

@tgalopin
Copy link
Contributor

tgalopin commented Dec 6, 2021

Thanks @weaverryan.

@tgalopin tgalopin merged commit 4fb8deb into symfony:2.x Dec 6, 2021
@duboiss duboiss mentioned this pull request Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants